Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

support new foundation module #44

Merged
merged 8 commits into from
Oct 28, 2022
Merged

Conversation

loin3
Copy link
Contributor

@loin3 loin3 commented Oct 27, 2022

As foundation module has big changes,

  • genesis has been changed
  • tx and query methods are added
  • test codes for foundation query are added

and since amino codec has been added for all lbm-sdk modules, in lbmjs only wasmplus, token, collection, foundation module's amino messages that are not supported in cosmjs are added.

CI will fail becuase new lbm and lbmjs-types are not published and lbmjs uses local lbmjs-types.

@loin3 loin3 requested review from zemyblue, 0Tech and dudong2 October 27, 2022 01:47
@loin3 loin3 requested a review from tkxkd0159 October 27, 2022 09:01
Copy link

@0Tech 0Tech left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you confirm that following suggestions make sense?

packages/finschia/src/modules/token/aminomessages.ts Outdated Show resolved Hide resolved
packages/finschia/src/modules/collection/aminomessages.ts Outdated Show resolved Hide resolved
packages/finschia/src/modules/collection/aminomessages.ts Outdated Show resolved Hide resolved
packages/finschia/src/modules/token/aminomessages.ts Outdated Show resolved Hide resolved
Copy link
Member

@zemyblue zemyblue left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please check the amino msgName with overlapping message names.

x/collection

  • MsgApprove: "lbm-sdk/collection/MsgApprove"
  • MsgModify: "lbm-sdk/collection/MsgModify"
  • MsgGrantPermission: "lbm-sdk/collection/MsgGrantPermission"
  • MsgRevokePermission: "lbm-sdk/collection/MsgRevokePermission"

x/token

  • MsgApprove: "lbm-sdk/token/MsgApprove"
  • MsgModify: "lbm-sdk/token/MsgModify"
  • MsgGrantPermission: "lbm-sdk/token/MsgGrantPermission"
  • MsgRevokePermission: "lbm-sdk/token/MsgRevokePermission"

Copy link
Member

@zemyblue zemyblue left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add the missing amino code of x/foundation

  • ThresholdDecisionPolicy
  • PercentageDecisionPolicy
  • ReceiveFromTreasuryAuthorization

https://github.com/line/lbm-sdk/blob/f682f758268c19dd93958abbbaf697f51e6991b3/x/foundation/codec.go#L34-L36

@loin3 loin3 requested review from zemyblue and 0Tech October 27, 2022 12:22
Copy link

@0Tech 0Tech left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@loin3 loin3 merged commit 4ed7d04 into main Oct 28, 2022
@loin3 loin3 deleted the chore/support-foundation-module branch October 28, 2022 03:05
@loin3 loin3 self-assigned this Oct 31, 2022
@zemyblue zemyblue mentioned this pull request Dec 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants